Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

test: fix test-repl timeout and tmpdir refresh #25425

Closed
wants to merge 1 commit into from
Closed

test: fix test-repl timeout and tmpdir refresh #25425

wants to merge 1 commit into from

Conversation

mscdex
Copy link
Contributor

@mscdex mscdex commented Jan 9, 2019 •
edited
Loading

I could not find a way to stress test this on the pi1 as armv6 (and armv7 for that matter) seems to not be a selectable target in Jenkins, although armv8/arm64 targets are?

Anyway, what prompted this was this recent CI failure on pi1-docker:

15:05:58 not ok 229 parallel/test-repl
15:05:58   ---
15:05:58   duration_ms: 7.962
15:05:58   severity: fail
15:05:58   exitcode: 1
15:05:58   stack: |-
15:05:58     out: ""
15:05:58     in: ""
15:05:58     out: "message"
15:05:58     in: "'Read, Eval, Print Loop'"
15:05:58     out: "invoke_me(987)"
15:05:58     in: "'invoked 987'"
15:05:58     out: "a = 12345"
15:05:58     in: "12345"
15:05:58     out: "{a:1}"
15:05:58     in: "{ a: 1 }"
15:05:58     out: "throw new Error('test error');"
15:05:58     in: "Thrown:"
15:05:58     in: "Error: test error"
15:05:58     out: "throw { foo: 'bar' };"
15:05:58     in: "Thrown: { foo: 'bar' }"
15:05:58     out: "function test_func() {"
15:05:58     /home/iojs/build/workspace/node-test-binary-arm/test/common/index.js:689
15:05:58     const crashOnUnhandledRejection = (err) => { throw err; };
15:05:58                                                  ^
15:05:58     
15:05:58     Error: The REPL did not reply as expected for:
15:05:58     
15:05:58     '... '
15:05:58         at Timeout.setTimeout [as _onTimeout] (/home/iojs/build/workspace/node-test-binary-arm/test/parallel/test-repl.js:854:14)
15:05:58         at listOnTimeout (timers.js:324:15)
15:05:58         at processTimers (timers.js:268:5)

The tmpdir addition was to allow running the test by itself, outside of a make test.

CI: https://ci.nodejs.org/job/node-test-pull-request/20036/

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@mscdex mscdex added repl Issues and PRs related to the REPL subsystem. test Issues and PRs related to the tests. labels Jan 9, 2019
@nodejs-github-bot
Copy link
Collaborator

@mscdex build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/2214/pipeline

@Trott
Copy link
Member

Trott commented Jan 9, 2019

I could not find a way to stress test this on the pi1 as armv6 (and armv7 for that matter) seems to not be a selectable target in Jenkins, although armv8/arm64 targets are?

The pi1 devices have their own separate stress test Jenkins jobs. https://ci.nodejs.org/view/All/job/node-stress-single-test-pi1-binary/ and https://ci.nodejs.org/view/All/job/node-stress-single-test-pi1-fanned/

@mscdex
Copy link
Contributor Author

mscdex commented Jan 9, 2019 •
edited
Loading

The pi1 devices have their own separate stress test Jenkins jobs. https://ci.nodejs.org/view/All/job/node-stress-single-test-pi1-binary/ and https://ci.nodejs.org/view/All/job/node-stress-single-test-pi1-fanned/

Ok, those do not seem to be listed under any existing tabs except "All". Could we add them (at least the fanned job) to the "Node.js" tab alongside the main stress test job that already exists there?

@Trott
Copy link
Member

Trott commented Jan 10, 2019

Ok, those do not seem to be listed under any existing tabs except "All". Could we add them (at least the fanned job) to the "Node.js" tab alongside the main stress test job that already exists there?

@nodejs/build ^^^^^^^

@mscdex
Copy link
Contributor Author

mscdex commented Jan 10, 2019

Also it looks like the pi1-fanned job actually executes on pi2 and pi3 machines in addition to pi1.

@mscdex
Copy link
Contributor Author

mscdex commented Jan 10, 2019 •
edited
Loading

Stress test on pi1: https://ci.nodejs.org/job/node-stress-single-test-pi1-binary/104/label=pi1-raspbian-stretch/

@refack
Copy link
Contributor

refack commented Jan 12, 2019

Ok, those do not seem to be listed under any existing tabs except "All". Could we add them (at least the fanned job) to the "Node.js" tab alongside the main stress test job that already exists there?

@nodejs/build ^^^^^^^

Does this make sense: https://ci.nodejs.org/view/Stress/

@mscdex
Copy link
Contributor Author

mscdex commented Jan 12, 2019

Does this make sense: https://ci.nodejs.org/view/Stress/

That is helpful, yes. Thanks.

@antsmartian antsmartian added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 15, 2019
@antsmartian
Copy link
Contributor

antsmartian commented Jan 17, 2019 •
edited
Loading

Landed in 7b6e9ae 🎉

pull bot pushed a commit to zys-contrib/node that referenced this pull request Jan 17, 2019
@mscdex @antsmartian
test: fix test-repl timeout and tmpdir refresh
7b6e9ae 
PR-URL: nodejs#25425
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
@mscdex mscdex deleted the test-repl-fixes branch January 17, 2019 19:05
addaleax pushed a commit that referenced this pull request Jan 23, 2019
@mscdex @addaleax
test: fix test-repl timeout and tmpdir refresh
ab86143 
PR-URL: #25425
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jan 24, 2019
Merged
BethGriggs pushed a commit that referenced this pull request Apr 29, 2019
@mscdex @BethGriggs
test: fix test-repl timeout and tmpdir refresh
637e876 
PR-URL: #25425
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
@BethGriggs BethGriggs mentioned this pull request May 1, 2019
Merged
BethGriggs pushed a commit that referenced this pull request May 10, 2019
@mscdex @BethGriggs
test: fix test-repl timeout and tmpdir refresh
2e9612f 
PR-URL: #25425
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
MylesBorins pushed a commit that referenced this pull request May 16, 2019
@mscdex @MylesBorins
test: fix test-repl timeout and tmpdir refresh
3fcc44d 
PR-URL: #25425
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
This was referenced May 29, 2019
Closed
Closed
Closed
Closed
@beevelop beevelop mentioned this pull request May 29, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BridgeAR BridgeAR BridgeAR approved these changes

@cjihrig cjihrig cjihrig approved these changes

@antsmartian antsmartian antsmartian approved these changes

@Trott Trott Trott approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. repl Issues and PRs related to the REPL subsystem. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@mscdex @nodejs-github-bot @Trott @refack @antsmartian @cjihrig @BridgeAR